Skip to content

Integrate Sigmond demo in Homepage and AI page - #494

Open
niravcodes wants to merge 23 commits into
mainfrom
nko/aidemo
Open

Integrate Sigmond demo in Homepage and AI page#494
niravcodes wants to merge 23 commits into
mainfrom
nko/aidemo

Conversation

@niravcodes

@niravcodes niravcodes commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Description

Adds Sigmond demo on the homepage and the SWML-> AI page

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Code cleanup / refactor

Related Issues

Testing

  • Added/updated unit tests
  • Tested manually
  • Tested with live SignalWire credentials (if applicable)

Checklist

  • I have read the CONTRIBUTING guidelines
  • My code follows the project's style guidelines
  • I have added tests for my changes (if applicable)
  • I have updated documentation (if applicable)
  • All existing tests pass

Additional Notes

Currently Fern sends:

Permissions-Policy camera=(), geolocation=()

to all it's response headers, thus auto-breaking every video call on Chrome-ish browsers. Firefox seems to ignore the header.

@niravcodes
niravcodes marked this pull request as ready for review July 21, 2026 18:54
@niravcodes niravcodes mentioned this pull request Jul 21, 2026
13 tasks
@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

@hey-august

hey-august commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Review (click to expand)

Reviewed PR #494 (nko/aidemo, local HEAD matches the PR head exactly, so this is the real merge candidate).

Verdict: two things I'd block on

1. The widget is loaded from a mutable @dev dist-tag. fern/components/sigmond-card/index.tsx:4

https://cdn.signalwire.com/npm/@signalwire/address-widget@dev/dist/address-widget.umd.js

dev currently resolves to 0.1.1-dev.20260608194727.42eacaa, and there are 18 published versions with a steady stream of dev builds. Anyone publishing a new dev build changes the docs homepage with no PR, no deploy, and no review. Worth knowing before you "just pin it": @dev is 519,304 bytes and the stable 0.1.1 is 472,814, so they are not the same code and swapping to latest is a behavior change, not a no-op. Ask Nirav which build the demo was actually tested against and pin that exact version.

2. A live embedded call token is hardcoded in the component. fern/components/sigmond-card/index.tsx:7

This is not a secret leak: I confirmed the token is served in the public page HTML on both preview pages, so an embed token is public by design. The problems are operational, and post-merge reversal costs a token rotation plus a docs deploy:

  • It is the first live credential in this repo. The only other eyJ… strings (fern/products/apis/pages/core/authorization.mdx:147, the browser-sdk v2 reference) are illustrative examples.
  • The payload is a JWE (dir / A256GCM), so expiry is not inspectable. Nobody can tell from the code when this stops working, and when it does the failure is a silent console.error behind a button that still looks clickable.
  • An anonymous "Call Sigmond" button on the docs homepage is an unmetered spend surface. There's no rate limiting or click throttle in the component beyond the per-element busy flag.

I'd want the token owner to confirm scope (destination-limited to /public/sigmond?), expiry, abuse controls, and who rotates it.

Good news: the blocker in the PR description is stale

The body says Fern sends Permissions-Policy: camera=(), geolocation=() and "auto-breaks every video call on Chrome-ish browsers." That is fixed. Both production and the PR preview now send:

permissions-policy: camera=(self), microphone=(self), geolocation=()

I also grepped the widget bundle for iframe creation and found none, so the widget runs same-origin and camera=(self) covers it. Update the description so nobody blocks on this. Since that header changed under the PR, Nirav should re-confirm one real call in Chrome on the preview.

Should fix, not blocking

  • aria-hidden="true" on the widget host (index.tsx:88). The entire live call UI mounts inside a container permanently marked aria-hidden, so a screen-reader user who activates the deliberately-accessible trigger (role="button", aria-label, Enter/Space handler) lands on a call they cannot perceive. Nothing moves focus into the call UI on open or returns it on close either.
  • The launcher and its wiring are separable, and the snippet makes that likely. The clickable markup lives in MDX while <SigmondWidget /> is a separate element that reaches out via document.querySelectorAll("[data-sigmond-launcher]"). Include snippets/common/talk-with-sigmond.mdx on a page and forget the component and you ship a card that looks like a button and does nothing. No build error, no runtime warning, and fern-md-check passes. Either have the component render the card, or console.warn when it finds zero launchers.
  • SigmondWidget is missing from the component catalog in .claude/rules/mdx-components.md, which lists every custom component and records VoiceWidget's "wrap usages in <llms-ignore>" rule. CLAUDE.md calls these rules binding.
  • Dead CSS: .sigmond-card-badge has three rules and no markup anywhere in the repo.
  • signalwire-address::part(launcher) { display: none !important; } is global and unscoped, tied to the widget's internal shadow-part name. A rename upstream puts an unstyled floating launcher on every docs page.
  • Teardown: host.remove() runs before unmount(mounted), so the widget tears down a detached node, and the void unmount(...) has no .catch(), giving an unhandled rejection on navigation.
  • Undocumented magic values inputVolume: 125 and autoGainControl: false deserve a one-line comment on why.

Nits

  • tabindex lowercase as a JSX prop. I confirmed it renders correctly in the served HTML, so it works, but React logs an "Invalid DOM property" warning.
  • The homepage band hand-rolls its own markup with a shortened copy of the snippet's description, so the two will drift.
  • The testing checklist claims unit tests were added and pass; the PR adds none.
  • .claude/rules/mdx-mechanics.md:49 asks for short-form icons (regular robot) over bare names, and this PR uses bare icon="video" / icon="arrow-right". Being fair, the two pre-existing Icon uses in the repo are also bare names, and the icons render fine as fas. Rule drift, not a PR problem.

What I verified vs. didn't

yarn fern-check passes (0 errors, 1 pre-existing warning). yarn fern-md-check passes (2825 files valid). On the live preview I confirmed the card and homepage band both render in the served HTML with no Unsupported JSX tag or client-content-error, the Icon SVGs render, and <llms-ignore> works correctly (the .md export of the AI page has zero Sigmond mentions). Both CDN URLs return 200.

I did not place an actual call, so the token's validity and the in-call UX are unverified. That's the one thing worth a live demo on the preview before merge.

Side note: this PR resolves an old worry I had recorded that Fern React components were not enabled for the signalwire org. They clearly are now, and I've corrected that note.

I am comfortable merging this. It works and looks way more integrated with the rest of the docs after a few design iterations. Some issues with the widget itself, but those live on a different repo. Claude argues to block on a couple items (above) but I'd be happy to see them filed and addressed later as a follow-up.

@hey-august
hey-august self-requested a review August 19, 2026 20:55

@hey-august hey-august left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At merge time, please add any outstanding review comments (non-blocking) to a new follow-up issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants